Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ERC: Signature Aggregation for ERC-4337 #626

Conversation

forshtat
Copy link
Contributor

@forshtat forshtat commented Sep 10, 2024

We're extracting the signature aggregator outside of ERC-4337, so we want first to create a new ERC.
PR for the new ERC is here: #627

@eip-review-bot
Copy link
Collaborator

eip-review-bot commented Sep 10, 2024

✅ All reviewers have approved.

ERCS/erc-4337-agg.md Outdated Show resolved Hide resolved
description: An ERC-4337 improvement to aggregation of all UserOperation signatures in a bundle
title: Signature Aggregation for Account Abstraction
author: Vitalik Buterin (@vbuterin), Yoav Weiss (@yoavw), Dror Tirosh (@drortirosh), Shahaf Nacson (@shahafn), Alex Forshtat (@forshtat)
discussions-to:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a discussions topic in Eth Magicians

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created.

@@ -0,0 +1,145 @@
---
eip:
description: An ERC-4337 improvement to aggregation of all UserOperation signatures in a bundle
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description should be after title

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@github-actions github-actions bot added the w-ci label Sep 18, 2024
Copy link

The commit a44004c (as a parent of dcc3d53) contains errors.
Please inspect the Run Summary for details.

@eip-review-bot eip-review-bot changed the title ERC-4337 extension: aggregated signatures Add ERC: Signature Aggregation for ERC-4334 Sep 19, 2024
@github-actions github-actions bot removed the w-ci label Sep 19, 2024
@eip-review-bot eip-review-bot changed the title Add ERC: Signature Aggregation for ERC-4334 Add ERC: Signature Aggregation for ERC-4337 Sep 19, 2024
@drortirosh
Copy link
Contributor

@abcoathup , what blocks this PR from being closed?

@abcoathup
Copy link
Contributor

@drortirosh you need an ERC editor to review (I am not an editor). There is only a small number of editors, so it can take some time to get reviewed. In the meantime, ensure that the validator doesn’t raise any errors & fix any issues.

There is a fortnightly office hours that you could attend: ethcatherders/EIPIP#360

Copy link
Collaborator

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the comments are fine to fix in draft. The only blocker is the external link.

ERCS/erc-7766.md Show resolved Hide resolved
ERCS/erc-7766.md Outdated Show resolved Hide resolved
ERCS/erc-7766.md Outdated
Comment on lines 108 to 109
* **code: -32506** - transaction rejected because wallet specified unsupported signature aggregator
* The `data` field SHOULD contain an `aggregator` value
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a touch vague I think.

ERCS/erc-7766.md Outdated
Comment on lines 128 to 130
## Reference Implementation

See `https://github.com/eth-infinitism/account-abstraction/tree/main/contracts`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still an external link. Please either copy your reference implementation to your assets/ directory, or remove this section entirely. Note that we cannot accept GPL'd code because it unfairly burdens implementers trying to follow your standard.

ERCS/erc-7766.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Be careful to always put inline code in backticks

@eip-review-bot eip-review-bot enabled auto-merge (squash) October 29, 2024 14:10
Copy link
Collaborator

@eip-review-bot eip-review-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Reviewers Have Approved; Performing Automatic Merge...

@eip-review-bot eip-review-bot merged commit 6fc4cf4 into ethereum:master Oct 29, 2024
10 checks passed
@drortirosh drortirosh deleted the AA-427-extract-aggregation-create-new branch October 30, 2024 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants